Skip to content

Conversation

@Leyza
Copy link
Contributor

@Leyza Leyza commented Nov 15, 2025

Why I did it

Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
  • Microsoft ADO (number only): 31626685

How I did it

Added vnet/vrf name field to the key for bgp peer range.

How to verify it

Tested on a physical testbed and UTs
image

Which release branch to backport (provide reason below if selected)

  • 202205
  • 202211
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@Leyza Leyza requested a review from qiluo-msft as a code owner November 15, 2025 01:12
Copilot AI review requested due to automatic review settings November 15, 2025 01:12
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for associating BGP peer ranges with VRF/VNET contexts in the YANG model. Previously, the BGP peer range model did not include fields to specify which VRF or VNET a peer range belongs to, even though this functionality was supported in the underlying system.

Key changes:

  • Added vrf_name field to BGP peer range configuration with support for both VRF and VNET references
  • Introduced composite key structure (vrf_name, peer_range_name) for the new BGP_PEER_RANGE_LIST
  • Maintained backward compatibility through separate BGP_PEER_RANGE_TEMPLATE_LIST without VRF/VNET association
  • Added comprehensive test coverage for valid VRF, valid VNET, and invalid VRF/VNET cases

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/sonic-yang-models/yang-models/sonic-bgp-peerrange.yang Added imports for sonic-vrf and sonic-vnet modules; introduced BGP_PEER_RANGE_LIST with vrf_name support while preserving BGP_PEER_RANGE_TEMPLATE_LIST for backward compatibility
src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json Updated test configurations to use template list for basic case; added test cases for valid VRF, valid VNET, and invalid VRF/VNET scenarios
src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json Added test case definitions with descriptions and expected validation results
src/sonic-yang-models/tests/files/sample_config_db.json Added sample configurations demonstrating BGP peer ranges with VRF and VNET associations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI review requested due to automatic review settings November 19, 2025 02:12
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

StormLiangMS
StormLiangMS previously approved these changes Nov 20, 2025
Copy link
Contributor

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StormLiangMS
Copy link
Contributor

@vaibhavhd could you take a look?

@vaibhavhd
Copy link
Contributor

I don't have any concerns with this change. However, @Leyza could you describe how this gap was detected? Is there a testcase to detect the gap here? Was there a config application failure due to the lack of this change on YANG model.

Also trying to understand why this was missed to begin with. How did we not catch this gap when the feature that added support for "Bgp peer range to associate with a vnet or vrf" was added?

@Leyza
Copy link
Contributor Author

Leyza commented Nov 21, 2025

@vaibhavhd We have a use case where we configure vnet bgp peer ranges, and that's where we noticed that the configuration did not pass yang validations, but functionality-wise everything is working. There will be test cases in the future that we are working on that will be applying vnet bgp peer ranges so this case will be covered there.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft qiluo-msft merged commit 472167b into sonic-net:master Nov 24, 2025
22 checks passed
@Leyza Leyza deleted the vnetBgpPeer branch November 25, 2025 08:58
gordon-nexthop pushed a commit to nexthop-ai/sonic-buildimage that referenced this pull request Nov 28, 2025
Why I did it
Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
Microsoft ADO (number only): 31626685
How I did it
Added vnet/vrf name field to the key for bgp peer range.

How to verify it
Tested on a physical testbed and UTs

Signed-off-by: gordon-nexthop <[email protected]>
ashutosh-agrawal pushed a commit to AnantKishorSharma/sonic-buildimage that referenced this pull request Nov 30, 2025
Why I did it
Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
Microsoft ADO (number only): 31626685
How I did it
Added vnet/vrf name field to the key for bgp peer range.

How to verify it
Tested on a physical testbed and UTs
hdwhdw pushed a commit to hdwhdw/sonic-buildimage that referenced this pull request Dec 2, 2025
Why I did it
Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
Microsoft ADO (number only): 31626685
How I did it
Added vnet/vrf name field to the key for bgp peer range.

How to verify it
Tested on a physical testbed and UTs
NavdhaJindal pushed a commit to NavdhaJindal/sonic-buildimage that referenced this pull request Dec 2, 2025
Why I did it
Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
Microsoft ADO (number only): 31626685
How I did it
Added vnet/vrf name field to the key for bgp peer range.

How to verify it
Tested on a physical testbed and UTs

Signed-off-by: Navdha Jindal <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202511: #24720

venu-nexthop pushed a commit to nexthop-ai/sonic-buildimage that referenced this pull request Dec 3, 2025
Why I did it
Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
Microsoft ADO (number only): 31626685
How I did it
Added vnet/vrf name field to the key for bgp peer range.

How to verify it
Tested on a physical testbed and UTs

Signed-off-by: venu-nexthop <[email protected]>
FengPan-Frank pushed a commit to FengPan-Frank/sonic-buildimage that referenced this pull request Dec 4, 2025
Why I did it
Bgp peer range yang model currently has no field to associate with a vnet or vrf, even though this feature is supported.

Work item tracking
Microsoft ADO (number only): 31626685
How I did it
Added vnet/vrf name field to the key for bgp peer range.

How to verify it
Tested on a physical testbed and UTs

Signed-off-by: Feng Pan <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to msft-202506: Azure/sonic-buildimage-msft#1875

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants